Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wpe-2.38 Improve memory usage during GCHeap snapshot #1444

Open
wants to merge 1 commit into
base: wpe-2.38
Choose a base branch
from
Open

wpe-2.38 Improve memory usage during GCHeap snapshot #1444

wants to merge 1 commit into from

Conversation

asurdej-comcast
Copy link

@asurdej-comcast asurdej-comcast commented Jan 13, 2025 •
edited by ews-bot-wpe-rdk
Loading

This is kind of improvement for GC heap snapshot generation that prints the snapshot directly to file instead keeping it all in json string (and then print into file). This helps to reduce memory usage spike that was required for huge json string and also eliminates a crash caused by string size limit exceed.

I think it can be useful for WPE, embedded case when memory is limited, not sure if suitable for upstream webkit repo.
With this patch I'm able to dump pretty big GC heap into file without getting OOM from kernel

The second change is to allow different dir for heap snapshot. With containers enabled /tmp space is limited usually so better to use other location, vendor specific

0
__libc_do_syscall
‎:
1
__pthread_kill_implementation
‎/usr/src/debug/glibc/2.35-r0/git/nptl/pthread_kill.c:43
2
raise
‎/usr/src/debug/glibc/2.35-r0/git/signal/../sysdeps/posix/raise.c:26
3
abort
‎/usr/src/debug/glibc/2.35-r0/git/stdlib/abort.c:79
4
void WTF::StringBuilder::allocateBuffer
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/WTF/wtf/text/StringBuilder.cpp:46
5
WTF::StringBuilder::extendBufferForAppendingWithUpconvert
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/WTF/wtf/text/StringBuilder.cpp:144
6
WTF::StringBuilder::appendQuotedJSONString
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/WTF/wtf/text/StringBuilderJSON.cpp:98
7
JSC::HeapSnapshotBuilder::json
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp:552
8
JSC::HeapSnapshotBuilder::json
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp:308
9
WebCore::GCController::dumpHeap
‎/usr/src/debug/wpe-webkit/2.38.8+gitAUTOINC+d893a3ed1d-r1/build/../git/Source/WebCore/bindings/js/GCController.cpp:162
10
_g_cclosure_marshal_VOID__OBJECT_OBJECT_ENUMv
‎/usr/src/debug/glib-2.0/1_2.72.3-r0/build/../glib-2.72.3/gio/gmarshal-internal.c:1380
11
_g_closure_invoke_va
‎/usr/src/debug/glib-2.0/1_2.72.3-r0/build/../glib-2.72.3/gobject/gclosure.c:893
12
g_signal_emit_valist
‎/usr/src/debug/glib-2.0/1_2.72.3-r0/build/../glib-2.72.3/gobject/gsignal.c:3406
13
g_signal_emit
‎/usr/src/debug/glib-2.0/1_2.72.3-r0/build/../glib-2.72.3/gobject/gsignal.c:3553
14
g_file_monitor_emit_event
‎/usr/src/debug/glib-2.0/1_2.72.3-r0/build/../glib-2.72.3/gio/gfilemonitor.c:294
```<!--EWS-Status-Bubble-Start-->
https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/38ae47ceffb7a2c00e24cf59c26bdcd55ff434e4

| Build-Tests | Layout-Tests |
| ----------- | ------------ |
| [✅ 🛠 wpe-238-amd64-build](https://ews-wpe-rdk.igalia.com/#/builders/1/builds/29 "Built successfully") | [✅ 🧪 wpe-238-amd64-layout](https://ews-wpe-rdk.igalia.com/#/builders/5/builds/18 "Passed tests") 
| [✅ 🛠 wpe-238-arm32-build](https://ews-wpe-rdk.igalia.com/#/builders/8/builds/28 "Built successfully") | [✅ 🧪 wpe-238-arm32-layout](https://ews-wpe-rdk.igalia.com/#/builders/6/builds/29 "Passed tests") 
| | 
| | 
<!--EWS-Status-Bubble-End-->

@emutavchi @asurdej-comcast
Improve memory usage during GCHeap snapshot
38ae47c 
1) Write GCHeap snapshot directly to file
2) Override GCHeap snapshot directory with EVN variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lawchingman lawchingman lawchingman approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@asurdej-comcast @lawchingman @emutavchi